Add default browser support - #26

Code review: default browser support

What this does

Adds a [browser] config section (default, profile, user_data_dir, proxy) and threads a managed-Chromium concept through the whole stack: a new core/Browser.cpp browser catalog with per-platform installation detection, ResolveManagedBrowserSession in the daemon that launches/reuses a CDP-enabled browser on an ephemeral port with a flock-based launch lock, a managed keyboard-driven navigation path in open_url, a config set-browser CLI verb, a Browser settings page in the tray, and Lua prelude updates. Roughly +1400/−160 across 20 files.

The overall shape is good — replacing the ad-hoc CdpUserDataDir() / candidate-list guessing with a single descriptor-driven resolution is a clear improvement, the DevToolsActivePort + ephemeral-port approach is the right way to avoid the fixed-9222 collision, and adding a cross-process launch lock is a real fix. Config validation is thorough and well-tested.

Main things I'd want addressed

Correctness

• open_url sends primary+n and then immediately types the URL with no wait for the new window to appear or take focus — a real race on the cold-start path, and the Lua code it replaces did wait. (DaemonDesktop.cpp:486)
• applicationName is the macOS bundle name on all platforms, so ListWindows never matches on Linux (google-chrome vs "Google Chrome"), breaking window binding and the tray's "is running" status. (DaemonDesktop.cpp:479)
• The URL is now delivered as synthetic keystrokes but is still only prefix-validated; control characters in it are no longer inert. (DaemonDesktop.cpp:488)
• Windows Chromium PATH lookup uses chromium.exe; the binary is chrome.exe. (Browser.cpp:121)
• IsExecutableFile doesn't check the execute bit. (Browser.cpp:24)

Behavior / design

• open_url now hard-fails on browser_launch_failed / browser_debug_unavailable / browser_config_invalid rather than falling back to a plain open — a regression for the simplest command in the API. (DaemonDesktop.cpp:509)
• The configured proxy is applied only to the default browser/profile pair; any other profile silently launches with no proxy. Security-relevant and not documented. (DaemonBrowser.cpp:872)
• managed.ensure no longer creates its own window and instead types into whatever is frontmost, diverging from the daemon path and leaving opts.createTimeoutMs dead. (LuaPrelude.cpp:1956)
• managed_submit_filled_proxy_auth decides to click "Sign in" from page-spoofable accessibility content. Worth gating behind an opt-in and anchoring on browser chrome rather than English prose. (LuaPrelude.cpp:1810)

Performance

• LoadAppConfig runs per request — ~75 TOML parses per managed bootstrap given the 200 ms browser_eval poll, and open_url loads it twice. (DaemonBrowser.cpp:821)
• BrowserCatalog() / DescribeBrowser() do filesystem and PATH probing synchronously on the wx UI thread on every wxEVT_CHOICE, and RefreshBrowserStatus additionally calls Platform::ListWindows there. Worth caching or moving off the UI thread.

Test coverage

Config validation, TOML/JSON round-trip, the CLI verb, NormalizeBrowserId, schema strings, and the Lua dry-run are all covered well. Gaps worth filling:

• Nothing exercises ManagedBrowserDataDir — neither the chrome/default legacy chrome-cdp special case nor the browser-profiles/<id>/<profile> layout, and nothing asserts PrepareManagedBrowserDataDir actually produces 0700.
• NormalizeBrowserId is tested only on inputs that match; the unrecognized-input passthrough (which feeds a path component) is untested.
• CliTests asserts --browser Safari --profile work round-trips, but the daemon silently ignores profile whenever the browser is unsupported. Either reject that combination or add a test pinning the intended behavior.

Style

Consistent with the surrounding code; the descriptor/session structs are clean and the error codes are registered in DaemonMetadata. Two small notes: the return AppDataDir(); after #endif in BrowserStorageRoot is unreachable on macOS and on the Linux branch, and the Lua-side browser_window_unavailable code isn't in the daemon's error-code list (fine if Lua-only, but worth confirming it's intentional).

Generated by Claude Code